-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[DirectX] Adding support for static samplers in yaml2obj/obj2yaml #139963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
bogner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than one comment about consistently using the ASSERT_* vs EXPECT_* macros in the tests.
| ASSERT_EQ(Sampler.MaxAnisotropy, 20u); | ||
| ASSERT_EQ(Sampler.ComparisonFunc, 4u); | ||
| ASSERT_EQ(Sampler.BorderColor, 0u); | ||
| EXPECT_FLOAT_EQ(Sampler.MinLOD, 4.56); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these be ASSERT_FLOAT_EQ? IIUC ASSERT_* stops immediately if the condition isn't met and EXPECT_* allow us to diagnose multiple issues. I don't think it makes sense to use one for the floating point fields and the other for the rest of the fields.
inbelic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Did you want something to track and update going through all the previously defined parameters and marking them as optional yaml parameters if applicable?
Closes: 126636